Skip to content

Conversation

Zylphrex
Copy link
Member

@Zylphrex Zylphrex commented Oct 8, 2025

Mixing the 2 into 1 type is confusing and this is in preparation to auto fetch the next page if the current page is empty which is possible with the flex time strategy.

@Zylphrex Zylphrex requested a review from a team as a code owner October 8, 2025 17:57
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Oct 8, 2025
};

return pageParamResult;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Flexible Pagination Overlaps With Cursor-Based

The getNextPageParam function's condition for flex time pagination (highFidelity || isFlexTimePageParam(pageParam)) is too broad. It now uses cursor-based pagination if highFidelity is true or if the previous page was flex time, even if isDescending or autoRefresh conditions don't warrant it. This can cause inconsistent pagination behavior.

Fix in Cursor Fix in Web

Comment on lines +311 to +314
const link = isGetPreviousPage ? links.previous : links.next;
if (!link?.results) {
return undefined;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential bug: Backward pagination is broken in 'flex time' mode due to incorrect handling of forward-only logic.
  • Description: In 'flex time' mode, which is strictly forward-only, the code attempts to handle previous page requests. This causes the pagination logic to return undefined because the API correctly reports no previous page is available, leading to broken backward pagination functionality.

  • Suggested fix: The code should explicitly handle the case where a previous page is requested in 'flex time' mode, either by preventing the request or by providing a clear state to the UI, instead of returning undefined and causing a silent failure.
    severity: 3.0, confidence: 5.0

Did we get this right? 👍 / 👎 to inform future reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants